-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add TypeScript build setup and update publish workflow #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Introduced a tsconfig.json for TypeScript compilation, updated package.json to output built files to the dist directory, and added build and clean scripts. The GitHub Actions publish workflow now builds the package before publishing. This prepares the package for distribution as compiled JavaScript with type definitions.
WalkthroughThis pull request sets up build infrastructure for the project. It adds TypeScript configuration, introduces pre-publish build steps in the workflow, updates package.json with build scripts and new entry points, and expands gitignore to exclude build artifacts and temporary files. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/package-publish.yml (1)
9-10: Update to latest action versions.
actions/checkout@v2andactions/setup-node@v2are outdated. Use v4 for checkout and v4 for setup-node.🔎 Suggested fix
- - uses: actions/checkout@v2 - - uses: actions/setup-node@v2 + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 with: node-version: '20.x' registry-url: 'https://registry.npmjs.org'
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
.github/workflows/package-publish.yml.gitignorepackage.jsontsconfig.json
🔇 Additional comments (4)
.gitignore (1)
56-58: LGTM! Build artifacts properly ignored.The additions correctly exclude IDE history and the new build output directory.
package.json (3)
3-3: Version bump looks correct.Incrementing to 1.0.1 for the build infrastructure change is appropriate.
5-6: Entry points correctly updated for built output.Main and types now point to compiled JavaScript and type definitions.
14-16: All essential package files are present.README.md, LICENSE, and package.json exist at the root. npm includes these files by default during publication, regardless of the
filesfield configuration.
| - name: Install Package | ||
| run: yarn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Add dependency caching to speed up workflow.
Caching node_modules reduces install time on subsequent runs.
🔎 Suggested fix
- uses: actions/setup-node@v4
with:
node-version: '20.x'
registry-url: 'https://registry.npmjs.org'
+ cache: 'yarn'
- name: Install Package
run: yarnCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/workflows/package-publish.yml around lines 14 to 15, the workflow
simply runs yarn without caching dependencies which slows repeated runs; add a
cache step using actions/cache (keying on OS + yarn.lock) to restore and save
Yarn cache (and optionally node_modules) before the Install Package step, then
run yarn install (or yarn --frozen-lockfile) so restored cache is used and
subsequent runs are much faster.
| - name: Build Package | ||
| run: yarn build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double build detected: prepublishOnly will rebuild.
The workflow runs yarn build, but npm publish (line 21) will trigger prepublishOnly, which runs clean && build again. This wastes time and could cause issues if the build isn't deterministic.
Solution: Remove the explicit build step here and rely on prepublishOnly:
🔎 Suggested fix
- name: Install Package
run: yarn
- - name: Build Package
- run: yarn build
- name: Publish
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
run: npm whoami && npm publish --access public📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Build Package | |
| run: yarn build | |
| - name: Install Package | |
| run: yarn | |
| - name: Publish | |
| env: | |
| NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} | |
| run: npm whoami && npm publish --access public |
🤖 Prompt for AI Agents
In .github/workflows/package-publish.yml around lines 16 to 17, the workflow
runs `yarn build` explicitly but `npm publish` invokes the `prepublishOnly`
lifecycle which already runs `clean && build`, causing a redundant second build;
remove the explicit `yarn build` step (the "Build Package" job/run) so the
workflow relies on `prepublishOnly` to perform the build before publish, keeping
any remaining steps and ensuring the publish step still runs `npm publish`.
| "types": "dist/index.d.ts", | ||
| "scripts": { | ||
| "test": "echo \"Error: no test specified\" && exit 1" | ||
| "test": "echo \"Error: no test specified\" && exit 1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Test script is still a placeholder.
The test script exits with an error. Add actual tests or remove the script.
🤖 Prompt for AI Agents
In package.json around line 8, the "test" script is just the placeholder that
echoes an error and exits; replace it with a real test command or remove it.
Install and configure your chosen test runner (e.g., jest, mocha, ava) and
update the "test" script to run that runner (or to run your test script via npm
run), or if you don't want tests yet, delete the "test" entry to avoid the
failing placeholder; ensure package.json scripts remain valid and any required
devDependencies are added to package.json.
| "test": "echo \"Error: no test specified\" && exit 1" | ||
| "test": "echo \"Error: no test specified\" && exit 1", | ||
| "build": "tsc", | ||
| "clean": "rm -rf dist", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean script breaks on Windows.
rm -rf dist only works on Unix systems. Use a cross-platform tool like rimraf.
🔎 Suggested fix
First, add rimraf to devDependencies:
"devDependencies": {
"rimraf": "^5.0.0",
...
}Then update the clean script:
- "clean": "rm -rf dist",
+ "clean": "rimraf dist",Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In package.json around line 10, the "clean" script uses "rm -rf dist" which
breaks on Windows; add rimraf as a devDependency (e.g., "rimraf": "^5.0.0") and
update the "clean" script to use rimraf (e.g., "clean": "rimraf dist") so the
command is cross-platform compatible.
| "typescript": "^5.0.0", | ||
| "@types/react": "19.0.0", | ||
| "react": "19.0.0", | ||
| "react-native": "0.81.0", | ||
| "react-native-reanimated": "3.19.4", | ||
| "react-native-gesture-handler": "2.24.0", | ||
| "react-native-svg": "15.11.1", | ||
| "expo-haptics": "12.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
React 19.0.0 React Native 0.81.0 compatibility
💡 Result:
Short answer: React Native 0.81 is intended to work with React 19 (the 19.x line), but some packages expect specific 19.y patch releases so you may hit peer-dependency conflicts if you use exactly 19.0.0. React Native releases around 0.80–0.82 were updated to the React 19 line (0.80/0.82 notes mention React 19 releases). Check and align your app and 3rd‑party libs' peerDependencies (or upgrade React to a later 19.x such as 19.1.x) to avoid conflicts. [1][2][3]
References
- React Native 0.81 release notes. [1]
- React Native 0.80 / 0.82 notes mentioning React 19 support. [2]
- Community reports of package peer-dependency conflicts when using [email protected]. [3]
Would you like me to check a specific package in your project for compatibility?
🌐 Web query:
react-native-reanimated 3.19.4 React Native 0.81 support requirements
💡 Result:
Short answer: Reanimated 3.19.4 supports React Native 0.81 (both Paper/old architecture and Fabric/new architecture) — see Reanimated compatibility tables. Requirements to note:
- Reanimated 3.19.x is listed as compatible with RN 0.81. [1]
- Reanimated 3 requires the "Paper" (old) architecture support and also has compatibility entries for Fabric; 3.19.x is supported for RN 0.81 in both matrices. [1]
- Build tooling: Android builds need CMake >= 3.22.1. [2]
- React Native 0.81 minimum platform/tooling requirements (affects your project): Node.js >= 20 and Xcode >= 16.1. (Upgrade RN to 0.81 per RN release notes.) [3]
Sources:
- Reanimated 3.x compatibility table. [turn0search2]
- Reanimated build docs (CMake requirement). [turn0search3]
- React Native 0.81 release notes (Node/Xcode minima). [turn0search6]
Consider upgrading React to 19.1.x or later.
React 19.0.0 and React Native 0.81.0 work together, but React 19.0.0 can cause peer-dependency conflicts with third-party packages. Upgrading to a later 19.x version (like 19.1.x) avoids these issues. react-native-reanimated 3.19.4 supports React Native 0.81 and works with both architectures.
🤖 Prompt for AI Agents
In package.json around lines 28 to 35, the pinned "react": "19.0.0" may cause
peer-dependency conflicts; update the React version to a compatible 19.1.x range
(for example "^19.1.0") and make sure related type and peer packages align
(update "@types/react" to a matching 19.1.x version or caret range), then run
your package manager (npm/yarn/pnpm) to install and run the app and test key
screens to ensure no regressions; commit the updated package.json and lockfile.
| "target": "es2018", | ||
| "module": "commonjs", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check React Native version and current tsconfig settings
echo "=== React Native version ==="
cat package.json | jq -r '.devDependencies["react-native"], .peerDependencies["react-native"]' 2>/dev/null || echo "Not found via jq"
echo -e "\n=== Current tsconfig.json ==="
cat tsconfig.jsonRepository: OneKeyHQ/react-native-animated-charts
Length of output: 764
Consider modernizing the TypeScript target and module format.
With React Native 0.81.0, you can safely use es2020 instead of es2018. Switching from commonjs to es2020 modules improves tree-shaking for consumers of your library. The current setup works fine, but these changes align better with your runtime and modern standards.
🤖 Prompt for AI Agents
In tsconfig.json around lines 3 to 4, update the TypeScript compilation settings
to modern targets by changing "target": "es2018" to "es2020" and "module":
"commonjs" to "es2020"; after updating, run a full build and unit/integration
tests to catch any module-format or runtime issues and adjust any import/export
usage or bundler configuration if needed.
| "noUnusedLocals": false, | ||
| "noUnusedParameters": false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Re-enable unused variable checks for better code quality.
Disabling noUnusedLocals and noUnusedParameters can hide dead code and accidental leftovers.
🔎 Suggested fix
- "noUnusedLocals": false,
- "noUnusedParameters": false,
+ "noUnusedLocals": true,
+ "noUnusedParameters": true,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "noUnusedLocals": false, | |
| "noUnusedParameters": false, | |
| "noUnusedLocals": true, | |
| "noUnusedParameters": true, |
🤖 Prompt for AI Agents
In tsconfig.json around lines 16 to 17, the compiler options have
"noUnusedLocals" and "noUnusedParameters" set to false; re-enable these checks
by changing both options to true so the TypeScript compiler will report unused
local variables and function parameters, helping identify dead code and
accidental leftovers; run the typechecker/tsc after the change and fix reported
unused symbols by removing or using them (or prefixing intentionally unused
parameters with _ and/or adjusting eslint/ts-ignore only where justified).
| "experimentalDecorators": true, | ||
| "emitDecoratorMetadata": true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for decorator usage patterns
rg -n '@\w+\(' src/ --type ts --type tsx -C2 || echo "No decorators found"Repository: OneKeyHQ/react-native-animated-charts
Length of output: 145
🏁 Script executed:
#!/bin/bash
# Verify tsconfig.json exists and show lines around 18-19
if [ -f tsconfig.json ]; then
echo "=== tsconfig.json lines 15-22 ==="
sed -n '15,22p' tsconfig.json
fi
# Broader decorator search (including other patterns)
echo -e "\n=== Searching for decorator patterns ==="
rg '@[A-Za-z]' src/ -C2 || echo "No @ patterns found in src/"
</script>Repository: OneKeyHQ/react-native-animated-charts
Length of output: 9617
Remove unused decorator flags.
experimentalDecorators and emitDecoratorMetadata are enabled but decorators aren't used anywhere in the codebase. Remove them to keep the config clean.
🤖 Prompt for AI Agents
In tsconfig.json around lines 18 to 19, the compilerOptions enable
"experimentalDecorators" and "emitDecoratorMetadata" though decorators are not
used in the repository; remove these two flags from compilerOptions to keep the
configuration minimal and avoid enabling unused language features, then run a
TypeScript build/tsc to verify no changes in behavior.
Summary by CodeRabbit
Infrastructure & Build
Chores
✏️ Tip: You can customize this high-level summary in your review settings.